-
Notifications
You must be signed in to change notification settings - Fork 226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable min/max macros from windows.h #356
Conversation
For what it's worth, I tried replacing some min(max(x,a),b) with std::clamp but both gcc and MSVC generate garbage assembly so I did not pursue the matter. Come on C++, where is the zero cost abstraction you always brag about :( |
Work around for another issue with MSVC that prevent using initializer lists when min/maxing 3 values. |
@TheGondos why do you say garbage? It looks totally fine and is half the size of the home-grown solution |
Home grown solution : 3ops, one memory access
std::clamp : 7ops, 5 memory accesses
I wouldn't call that totally fine.
MSVC produces optimal code with the manual clamping (although if it were me I would probably do the load from memory first) |
@TheGondos oh sorry, for some reason they are reversed in the assembly window. I didn't look at the labels and thought the top part was for the clamp version. Nevermind. |
Clang on the other hand does well on both: https://godbolt.org/z/Pre1r81jf |
@TheGondos think you missed an |
Let me check, it builds cleanly on my side. I'll rebase to see if it's a conflict
Isn't it supposed to be downloaded automatically?
cmake, my favorite pile of steaming mess... |
I manually put the files for irrKlang, it should be OK. I forced pushed a rebase from the latest main. |
@TheGondos unfortunately this PR had unintended consequences for addon developers. Since you've made changes to the There is a pretty simple fix though -- just wrap all - foo = std::max(bar, baz);
+ foo = (std::max)(bar, baz); |
Nevermind, I've just sent a PR #365 myself |
This PR is about removing the min/max macros that are defined by default in windows.h.
Using the /DNOMINMAX flag, they are not defined and we can use the functions from the C++ library.
Impacts :